Repository navigation
feat(boto3): Add SQS extension - #8137
pabloDeputter wants to merge 8 commits into
Conversation
Coverage Results 📊✅ 57980 passed | ⏭️ 3008 skipped | Total: 60988 | Pass Rate: 95.07% | Execution Time: 161m 57s 📊 Comparison with Base Branch
All tests are passing successfully. ✅ Patch coverage is 100.00% (69 of 69 changed executable lines covered; target 80%). Changed files with executable lines (5)
Coverage diff@@ Coverage Diff @@
## master #8137 +/-##
==========================================
+ Coverage 90.35% 90.41% +0.06%
==========================================
Files 203 199 -4
Tracked lines 26931 21365 -5566
Branches 10058 7398 -2660
==========================================
- Hits 24331 19316 -5015
+ Misses 2600 2049 -551
+ Partials 1517 1234 -283Generated by Coverage Action |
5f4f1d0 to
0f90423
Compare
### Description Removes `AwsCallContext.service_id_hyphenized` from `sentry_sdk/integrations/boto3/_context.py`. It's still computed on every call (`service_id.hyphenize()`), but on `major/3.0` nothing reads it. It had two readers. One was the `aws.service_id` span tag, which #7261 ("Drop tags auto-set by the SDK") removed. The other was the span name: #7800 ("align span names with OTel conventions") switched that to `ctx.service_id`, so the hyphenized value lost its last reader. This removes the slot, the attribute and the `hyphenize()` call. **Evidence** - On `major/3.0`, `git grep -w service_id_hyphenized` matches only the three lines in `_context.py` that set it up. - The open boto3 extension PRs against `major/3.0` (#8117 SQS, #8137 Lambda) don't reference it. On `master`, `service_id_hyphenized` is still used for the span name and the tag, so this only applies to `major/3.0`. #### Issues n/a <!-- junior-request-attribution:start --> via **Ivana Kellyer**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:slack%3AC0BTREZTS2Z%3A1793270559.842119 --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/slack%3AC0BTREZTS2Z%3A1793270559.842119) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC0BTREZTS2Z%3A1793270559.842119/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Ivana Kellyer <ivana.kellyer@sentry.io>
| attributes[SPANDATA.AWS_SQS_QUEUE_URL] = queue_url | ||
|
|
||
| # `https://sqs.eu-north-1.amazonaws.com/123456789012/orders` => ['123456789012', 'orders'] | ||
| path = unquote(urlsplit(queue_url).path).strip("/").split("/") |
There was a problem hiding this comment.
Is queue_url validated by the library before it's handed to us through ctx.params?
There was a problem hiding this comment.
No, all parameters used in get_request_attributes() are yet to be validated. QueueUrl is also only checked to be a string, e.g. a non-string 1234 will raise an error at urlsplit and is captured at boto3/_instrumentation.py=180, so all SQS request attributes are dropped. Then get_span_name() at boto3/_instrumentation.py:183 will raise an error (if it's one of the API operations in _MESSAGING_OPERATIONS), which is captured again at boto3/_client.py:92, so no span is sent. Other operations still send the span, just without any SQS attributes.
There was a problem hiding this comment.
I did fix this partly in #8142, by adding another guard to get_span_name() at boto3/_instrumentation.py=183, so the span is sent, but still has no request attributes.
So I added some more precautions (c640799) in get_span_name() so that it doesn't take the operation type from attributes anymore, and the queue_url parsing is also guarded.
There was a problem hiding this comment.
Summary for whoever resolves this thread. botocore only checks that QueueUrl is a string, so this PR no longer relies on its shape:
- c640799 (this PR):
get_span_name()takes the operation type from_MESSAGING_OPERATIONSinstead of the request attributes, and the URL parsing inget_request_attributes()sits insidecapture_internal_exceptions(). A malformed URL now only loses the destination name and account ID, and the span is still sent. - c73d9ba (feat(boto3): Add SNS extension #8142): a second
capture_internal_exceptions()guard aroundget_span_name()in_instrumentation.py.
I merged major/3.0 into this branch today, and the boto3 tests still pass.
- stop reading operation type from attributes in `get_span_name()` so that failure in `get_request_attributes()` can't make this fail as well - wrap URL parsing in `get_request_attributes()` so failure just loses the attributes that come from url
…sqs-extension Co-Authored-By: Johannes Daxböck <114897+dingsdax@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f6c8099. Configure here.
Botocore validates params after our hook, so a bad Entries or ARN value now only loses that attribute, not every SQS request attribute. Move the batch error.type code to _utils so SNS can share it, and link the SQS docs for BatchResultErrorEntry. Co-Authored-By: Johannes Daxböck <114897+dingsdax@users.noreply.github.com>
sentry-junior
Bot
changed the title
Description
Adds the SQS extension, now SQS MESSAGING operations become
queue.*spans so they show up in the Queues insight; all other SQS APIs stayrpc:SendMessage,SendMessageBatch:queue.publish, kindproducer(default isclient), namedsend <queue>ReceiveMessage:queue.receive, namedreceive <queue>DeleteMessage,DeleteMessageBatch:queue.settle, namedsettle <queue>Adds the following attributes:
messaging.system(aws_sqs),messaging.operation.typeandmessaging.operation.nameon the messaging APIs (all from OTel)messaging.batch.message_counton batch APIs: submitted entries for send/delete, returned messages for receivemessaging.message.idonSendMessage, and onReceiveMessagewhen exactly one message is returnedaws.sqs.queue.urlandmessaging.destination.name(queue name) on every API withQueueUrl; destination name also fromQueueNameandSourceArnIssues
Resolves #7572 & PY-2803